-
-
Notifications
You must be signed in to change notification settings - Fork 59
[TYPES] [Features] Added creation of JavaScript password to encrypt vendor scripts #3132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Added creation of javascript password to encrypt vendor scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for vendor-specific JavaScript password encryption in the ioBroker setup process. The changes enable vendors to provide a JavaScript password in their vendor configuration file (iob-vendor.json), which will be encrypted and stored in the system configuration.
- Added
javascriptPasswordfield to vendor file interface and handling logic - Improved type safety across setup files with better TypeScript typing
- Updated database configuration types to support Redis Sentinel with array types for host and port
- Applied code modernization using nullish coalescing operators and optional chaining
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/types-dev/config.d.ts | Updated DatabaseOptions interface to support Redis Sentinel (arrays for host/port) and made dataDir optional |
| packages/cli/src/lib/setup/setupVendor.ts | Added JavaScript password encryption feature, improved typing with iobVendorFile interface and InternalLogger type |
| packages/cli/src/lib/setup/setupSetup.ts | Enhanced type safety, improved import organization, and updated variable naming consistency |
| CHANGELOG.md | Documented the new JavaScript password encryption feature |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.
|
|
||
| if (obj?.native) { | ||
| const javascriptPasswordEncrypted = tools.encrypt(obj.native.secret, javascriptPassword); | ||
| if (obj.native?.javascriptPassword !== javascriptPasswordEncrypted) { |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition on line 249 checks obj.native?.javascriptPassword but obj.native was already checked for truthiness on line 247. This redundant optional chaining is unnecessary since we already know obj.native exists at this point.
| if (obj.native?.javascriptPassword !== javascriptPasswordEncrypted) { | |
| if (obj.native.javascriptPassword !== javascriptPasswordEncrypted) { |
| if (obj?.native) { | ||
| const javascriptPasswordEncrypted = tools.encrypt(obj.native.secret, javascriptPassword); | ||
| if (obj.native?.javascriptPassword !== javascriptPasswordEncrypted) { | ||
| obj.native ||= {}; |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 250 assigns obj.native ||= {} but this is inside a block where obj.native has already been verified to exist (line 247 checks if (obj?.native)). This assignment is redundant and will never execute because obj.native is guaranteed to be truthy here.
| obj.native ||= {}; |
| for (const sHost of config.states.host) { | ||
| if (tools.isListenAllAddress(sHost)) { | ||
| config.states.host = ipHost ?? ''; | ||
| } | ||
| } | ||
| for (const oHost of config.objects.host) { | ||
| if (tools.isListenAllAddress(oHost)) { | ||
| config.objects.host = ipHost ?? ''; | ||
| } |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic in lines 332-341 has an issue: when iterating over the host arrays, if any single host is "listen all", the entire config.states.host or config.objects.host is replaced with a single string (ipHost), losing the array of other hosts. This should either update only the matching element in the array or handle the conversion differently to avoid data loss.
| for (const sHost of config.states.host) { | |
| if (tools.isListenAllAddress(sHost)) { | |
| config.states.host = ipHost ?? ''; | |
| } | |
| } | |
| for (const oHost of config.objects.host) { | |
| if (tools.isListenAllAddress(oHost)) { | |
| config.objects.host = ipHost ?? ''; | |
| } | |
| if (Array.isArray(config.states.host)) { | |
| config.states.host = config.states.host.map((sHost: string) => | |
| tools.isListenAllAddress(sHost) ? (ipHost ?? '') : sHost | |
| ); | |
| } | |
| if (Array.isArray(config.objects.host)) { | |
| config.objects.host = config.objects.host.map((oHost: string) => | |
| tools.isListenAllAddress(oHost) ? (ipHost ?? '') : oHost | |
| ); |
| console.log('Config ok. Please restart ioBroker: "iobroker restart"'); | ||
| callback(); | ||
| } else { | ||
| // Find is any of the hosts is "listen all" or reachable |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: "Find is any" should be "Find if any".
| // Find is any of the hosts is "listen all" or reachable | |
| // Find if any of the hosts is "listen all" or reachable |
| if (!Object.prototype.hasOwnProperty.call(config, 'dataDir')) { | ||
| // Workaround: there was a bug with admin v5 which could remove the dataDir attribute -> fix this | ||
| // TODO: remove it as soon as all adapters are fixed which use systemConfig.dataDir, with v5.1 we can for sure remove this | ||
| config.dataDir = tools.getDefaultDataDir(); | ||
| fs.writeJSONSync(configFileName, config, { spaces: 2 }); | ||
| } |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check if (!Object.prototype.hasOwnProperty.call(config, 'dataDir')) on line 1613 no longer has any meaningful action inside the block since the assignment config.dataDir = tools.getDefaultDataDir() was removed. Either the entire check should be removed, or the assignment should be restored if the workaround is still needed. Writing the config file without actually modifying it serves no purpose.
Added creation of javascript password to encrypt vendor scripts
Vendor wants to encrypt all specific scripts.